Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move -install_name linker flag from the {ios,tvos}_framework macro into the rule implementation. #1207

Conversation

thii
Copy link
Member

@thii thii commented Aug 4, 2021

PiperOrigin-RevId: 382086173
(cherry picked from commit 172ad76)

Conflicts:
apple/internal/ios_rules.bzl
apple/internal/tvos_rules.bzl
apple/ios.bzl
apple/tvos.bzl

@google-cla
Copy link

google-cla bot commented Aug 4, 2021

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

@google-cla google-cla bot added the cla: no label Aug 4, 2021
thii referenced this pull request Aug 4, 2021
…o into the rule implementation.

PiperOrigin-RevId: 382086173
@thii thii force-pushed the move-install_name-linker-flag-from-the-ios-tvos-_framework-macro-into-the-rule-implementation branch from e75e115 to 43708ba Compare August 6, 2021 00:55
@google-cla
Copy link

google-cla bot commented Aug 6, 2021

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

@thii
Copy link
Member Author

thii commented Aug 6, 2021

Depends on #1188

@google-cla google-cla bot added cla: yes and removed cla: no labels Nov 10, 2021
@brentleyjones brentleyjones force-pushed the move-install_name-linker-flag-from-the-ios-tvos-_framework-macro-into-the-rule-implementation branch from 43708ba to d72ca65 Compare November 23, 2021 21:39
@brentleyjones brentleyjones changed the base branch from master to segiddins/0b57552b8edb22539b865ed744fb9106b0be3f87 November 23, 2021 21:39
@lyft-lint-bot
Copy link

Lyft integration job started: https://buildkite.com/lyft/rules-apple/builds/170 (must be Lyft employee to view)

@@ -680,6 +665,25 @@ def _ios_framework_impl(ctx):
res_attrs = ["resources"],
)

extra_linkopts = [
"-dynamiclib",
"-Wl,-install_name,@rpath/{name}{extension}/{name}".format(
Copy link
Collaborator

@brentleyjones brentleyjones Nov 23, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I used this version to work around https://github.com/bazelbuild/bazel/blob/1c3a2456c95fd19974a5b2bd33c5ebdb2b2277e4/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkCommandLine.java#L295 causing an issue. Think this is bug worthy?

One nice thing about doing it this way: it's the same argument that was being sent before.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Yes I think we should report it.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@thii thii marked this pull request as ready for review November 23, 2021 23:10
@brentleyjones brentleyjones deleted the branch bazelbuild:master November 23, 2021 23:43
@brentleyjones
Copy link
Collaborator

Hmm, it should have re-branched, not closed this...

@brentleyjones brentleyjones reopened this Nov 23, 2021
@brentleyjones brentleyjones changed the base branch from segiddins/0b57552b8edb22539b865ed744fb9106b0be3f87 to master November 23, 2021 23:46
allevato and others added 2 commits November 23, 2021 18:08
…o into the rule implementation.

PiperOrigin-RevId: 382086173
(cherry picked from commit 172ad76)

This also changes the arguments to use `-Wl,-install_name,@rpath` to work around an issue in bazel: https://github.com/bazelbuild/bazel/blob/1c3a2456c95fd19974a5b2bd33c5ebdb2b2277e4/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkCommandLine.java#L295
@brentleyjones brentleyjones force-pushed the move-install_name-linker-flag-from-the-ios-tvos-_framework-macro-into-the-rule-implementation branch from d72ca65 to b43a777 Compare November 24, 2021 00:08
@lyft-lint-bot
Copy link

Lyft integration job started: https://buildkite.com/lyft/rules-apple/builds/172 (must be Lyft employee to view)

@brentleyjones brentleyjones merged commit 18fcf6a into bazelbuild:master Nov 24, 2021
@thii thii deleted the move-install_name-linker-flag-from-the-ios-tvos-_framework-macro-into-the-rule-implementation branch November 24, 2021 02:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants